Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Expand String.EndsWith/MemoryExtensions.EndsWith in JIT #98593

Merged
merged 6 commits into from
Feb 20, 2024
Merged

Expand String.EndsWith/MemoryExtensions.EndsWith in JIT #98593

merged 6 commits into from
Feb 20, 2024

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Feb 17, 2024 •
edited
Loading

It turns out it's not difficult to extend the existing *.StartsWith expansion to cover *.EndsWith as well. Example:

const string corelib = "System.Private.CoreLib.dll";

bool Test() => str.EndsWith(corelib, StringComparison.OrdinalIgnoreCase);

// Same for Span<char>/ReadOnlySpan<char> APIs

Old codegen:

; Assembly listing for method Program:Test
       mov      rdx, 0x20D80109480 ; 'System.Private.CoreLib.dll'
       mov      r8d, 5
       cmp      dword ptr [rcx], ecx
       tail.jmp [System.String:EndsWith(System.String,int):ubyte:this]

New codegen:

; Assembly listing for method Program:Test
       cmp      dword ptr [rcx+0x08], 26
       jl       SHORT G_M64019_IG04
       mov      eax, dword ptr [rcx+0x08]
       lea      rax, bword ptr [rcx+2*rax-0x34]
       vmovups  ymm0, ymmword ptr [rax+0x0C]
       vpor     ymm0, ymm0, ymmword ptr [reloc @RWD00]
       vpxor    ymm0, ymm0, ymmword ptr [reloc @RWD32]
       vmovups  ymm1, ymmword ptr [rax+0x20]
       vpor     ymm1, ymm1, ymmword ptr [reloc @RWD64]
       vpxor    ymm1, ymm1, ymmword ptr [reloc @RWD96]
       vpor     ymm0, ymm1, ymm0
       vptest   ymm0, ymm0
       sete     al
       movzx    rax, al
       jmp      SHORT G_M64019_IG05
G_M64019_IG04:
       xor      eax, eax
G_M64019_IG05:
       movzx    rax, al
       vzeroupper 
       ret      
RWD00  	dq	0020002000200020h, 0020000000200020h, ...
RWD32  	dq	0074007300790073h, 0070002E006D0065h, ...
RWD64  	dq	0020002000200020h, 0020002000200000h, ...
RWD96  	dq	0065007400610076h, 0072006F0063002Eh, ...

@ghost ghost assigned EgorBo Feb 17, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 17, 2024
@ghost
Copy link

ghost commented Feb 17, 2024

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

It turns out it's not difficult to extend the existing *.StartsWith expansion to cover *.EndsWith as well. Affected APIs:

const string corelib = "System.Private.CoreLib.dll";

bool Test() => str.EndsWith(corelib, StringComparison.OrdinalIgnoreCase);

// Same for Span<char>/ReadOnlySpan<char> APIs

Old codegen:

; Assembly listing for method Program:Test
       mov      rdx, 0x20D80109480 ; 'System.Private.CoreLib.dll'
       mov      r8d, 5
       cmp      dword ptr [rcx], ecx
       tail.jmp [System.String:EndsWith(System.String,int):ubyte:this]

New codegen:

; Assembly listing for method Program:Test
       cmp      dword ptr [rcx+0x08], 26
       jl       SHORT G_M64019_IG04
       mov      eax, dword ptr [rcx+0x08]
       lea      rax, bword ptr [rcx+2*rax-0x34]
       vmovups  ymm0, ymmword ptr [rax+0x0C]
       vpor     ymm0, ymm0, ymmword ptr [reloc @RWD00]
       vpxor    ymm0, ymm0, ymmword ptr [reloc @RWD32]
       vmovups  ymm1, ymmword ptr [rax+0x20]
       vpor     ymm1, ymm1, ymmword ptr [reloc @RWD64]
       vpxor    ymm1, ymm1, ymmword ptr [reloc @RWD96]
       vpor     ymm0, ymm1, ymm0
       vptest   ymm0, ymm0
       sete     al
       movzx    rax, al
       jmp      SHORT G_M64019_IG05
G_M64019_IG04:
       xor      eax, eax
G_M64019_IG05:
       movzx    rax, al
       vzeroupper 
       ret      
RWD00  	dq	0020002000200020h, 0020000000200020h, ...
RWD32  	dq	0074007300790073h, 0070002E006D0065h, ...
RWD64  	dq	0020002000200020h, 0020002000200000h, ...
RWD96  	dq	0065007400610076h, 0072006F0063002Eh, ...
Author: EgorBo
Assignees: EgorBo
Labels:

area-CodeGen-coreclr
Milestone: -

@build-analysis build-analysis bot mentioned this pull request Feb 17, 2024
Closed
@EgorBo
Copy link
Member Author

EgorBo commented Feb 17, 2024

@MihuBot

@MihuBot MihuBot mentioned this pull request Feb 17, 2024
Open
@EgorBo EgorBo marked this pull request as ready for review February 17, 2024 15:34
This was referenced Feb 17, 2024
Closed
Open
@EgorBo
Copy link
Member Author

EgorBo commented Feb 19, 2024

@AndyAyersMS @jakobbotsch @dotnet/jit-contrib PTAL. Simple change to extend StartsWith expansion with EndsWith. Basically, EndsWith is SequenceEquals where we shift the offset to dataPtr + (stringLength * 2 - constValueLength * 2)

@EgorBo
Copy link
Member Author

EgorBo commented Feb 19, 2024

Actually, some of the unrolling can probably be removed entirely since we already have a generic SequenceEquals unrolling in a late phase. But it's still needed for a bit complicated OrdinalIgnoreCase

jakobbotsch
jakobbotsch approved these changes Feb 19, 2024
View reviewed changes
Copy link
Member

@jakobbotsch jakobbotsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM otherwise

EgorBo and others added 2 commits February 20, 2024 15:41
@EgorBo @jakobbotsch
Apply suggestions from code review
8e294b2 
Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>
@EgorBo
enum class
c9c0c24
@build-analysis build-analysis bot mentioned this pull request Feb 20, 2024
Open
3 tasks
@EgorBo
Copy link
Member Author

EgorBo commented Feb 20, 2024

CI Failure is #98406

@EgorBo EgorBo merged commit 46870a0 into dotnet:main Feb 20, 2024
195 of 197 checks passed
@EgorBo EgorBo deleted the endswith branch February 20, 2024 18:37
@DrewScoggins DrewScoggins mentioned this pull request Feb 27, 2024
Closed
@matouskozak matouskozak mentioned this pull request Feb 28, 2024
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@SingleAccretion SingleAccretion SingleAccretion left review comments

@jakobbotsch jakobbotsch jakobbotsch approved these changes

@AndyAyersMS AndyAyersMS Awaiting requested review from AndyAyersMS

Assignees

@EgorBo EgorBo

Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@EgorBo @jakobbotsch @SingleAccretion